Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

🐛 Ignore collection failures in non-tests #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

webknjaz
Copy link

@webknjaz webknjaz commented Aug 17, 2021

This change applies the pre-existing patterns to identify if the files with collection problems are tests. It is then used to eliminate the false-positives of F6401 (cannot-enumerate-pytest-fixtures).

As a side effect, this patch also includes precise file paths that may be used to reproduce the problem.

Fixes #20
Fixes #21

@webknjaz
Copy link
Author

Hey @nedbat, could you check this PR in the context of your report #20?

webknjaz referenced this pull request in nedbat/scriv Aug 17, 2021
Copy link

@ssbarnea ssbarnea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks ok but please remove emoji from PR before merging it. I have nightmares about emoji in git changelog.

webknjaz added a commit to sanitizers/octomachinery that referenced this pull request Aug 18, 2021
@whg517
Copy link

whg517 commented Aug 30, 2021

I hope this feature will be incorporated soon because my F6401 is really annoying me. Sometimes it just show cannot-enumerate-pytest-fixtures and I don't know what went wrong,

Copy link
Owner

@reverbc reverbc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making the PR. I validate it and it should avoid raising warnings in unrelated modules.

Also please help to note this in CHANGELOG.md

if (ret != pytest.ExitCode.OK or legitimate_failure_paths) and is_test_module:
self.add_message(
'cannot-enumerate-pytest-fixtures',
args=' '.join(legitimate_failure_paths | {node.file}),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found that there's a potential duplication from this line: legitimate_failure_paths can contain relative paths, but the node.file is in absolute path. In my env it's throwing out pytest warning with duplicated paths:

sandbox/test_parent.py:1:0: F6401: pylint-pytest plugin cannot enumerate and collect pytest fixtures. Please run `pytest --fixtures --collect-only sandbox/test_parent.py /Users/reverbc/Workspace/pylint-pytest/sandbox/test_parent.py` and resolve any potential syntax error or package dependency issues (cannot-enumerate-pytest-fixtures)

Where the sandbox/test_parent.py and /Users/reverbc/Workspace/pylint-pytest/sandbox/test_parent.py are pointing to the same file.

Can you help to unify the path strings, maybe with str(Path(...).absolute()) for both collection_report.nodeid and node.file?

@reverbc reverbc mentioned this pull request Aug 30, 2021
3 tasks
@nedbat
Copy link

nedbat commented Aug 31, 2021

@webknjaz thanks, this does fix my issue.

bruno-fs added a commit to bruno-fs/quipucords that referenced this pull request Feb 24, 2022
When  reverbc/pylint-pytest#22 gets merged we might
reenable this check.
bruno-fs added a commit to bruno-fs/quipucords that referenced this pull request Feb 24, 2022
When  reverbc/pylint-pytest#22 gets merged we might
reenable this check.
bruno-fs added a commit to bruno-fs/quipucords that referenced this pull request Mar 2, 2022
When  reverbc/pylint-pytest#22 gets merged we might
reenable this check.
bruno-fs added a commit to bruno-fs/quipucords that referenced this pull request Mar 3, 2022
When  reverbc/pylint-pytest#22 gets merged we might
reenable this check.
@huxuan
Copy link

huxuan commented Mar 28, 2022

Any further progress on this? Really appreciate if this mr can be merged with a new version released.

@nedbat
Copy link

nedbat commented Mar 28, 2022

I've confirmed that this pull request prevents the errors I reported.

@marcgibbons
Copy link

@reverbc Can this get merged & published?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG]Why does adding a module produce F6401 0.3.0 found my fixtures; 1.1.2 cannot.
7 participants